Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retry subscription setup if necessary #873

Merged
merged 7 commits into from
Sep 4, 2024
Merged

Retry subscription setup if necessary #873

merged 7 commits into from
Sep 4, 2024

Conversation

agners
Copy link
Collaborator

@agners agners commented Sep 3, 2024

With the update of the Matter SDK wheels to 2024.7.1 (#809) the SDK no longer auto-resubscribes when the read of the subscription fails (see project-chip/connectedhomeip#34370). The Server had a comment that it relies on mDNS rediscovery to re-establish the subscription. But this is not always the case. This change adds a retry mechanism to the subscription setup for a fixed period of time after seeing it via mDNS.

Additionally, it avoids recreating the subscription if the liveness timeout did not occur yet. This puts more trust into the SDK handling the subscriptions.

Fixes: home-assistant/core#124503

@agners
Retry subscription setup if necessary
4902d4a 
With the update of the Matter SDK wheels to 2024.7.1 (#809) the SDK
no longer auto-resubscribes when the read of the subscription fails
(see project-chip/connectedhomeip#34370).
The Server had a comment that it relies on mDNS rediscovery to
re-establish the subscription. But this is not always the case. This
change adds a retry mechanism to the subscription setup for a fixed
period of time after seeing it via mDNS.

Additionally, it avoids recreating the subscription if the liveness
timeout did not occur yet. This puts more trust into the SDK handling
the subscriptions.
@agners agners added the bugfix Pull request that fixes a (known) issue/bug label Sep 3, 2024
agners and others added 3 commits September 3, 2024 21:12
@agners @marcelveldt
Update matter_server/server/device_controller.py
aa5622e 
Co-authored-by: Marcel van der Veldt <[email protected]>
@agners
Create task to setup node
3b239a6 
Use a task in all cases when we (re-)setup the node. Use the task to
also make sure that only a single setup is running at a time. And
finally it allows us to abort any currently running setup task at the
end.
@agners
Improve logging
66faa0b
@agners
Cancel setup task on node removal
69a20c4
@agners
Add comment
2ab5604
@agners
Rename _setup_tasks_with_retry and _setup_tasks
cad00c4 
We'll have to implement retry on our end, so use this as the new normal
naming.
marcelveldt
marcelveldt approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@agners agners merged commit 68754b0 into main Sep 4, 2024
4 checks passed
@agners agners deleted the retry-subscription-if-necessary branch September 4, 2024 07:46
@agners agners mentioned this pull request Sep 4, 2024
Merged
@agners agners mentioned this pull request Sep 12, 2024
Merged
@Drealine
Copy link

Still got the same issue for me :

[139815571515872] Unexpected exception [139815779709520] Unexpected exception [139815459890672] Unexpected exception Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 245, in handle_call_service response = await hass.services.async_call( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/core.py", line 2761, in async_call response_data = await coro ^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/core.py", line 2804, in _execute_service return await target(service_call) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 996, in entity_service_call single_response = await _handle_entity_call( ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 1068, in _handle_entity_call result = await task ^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/components/lock/__init__.py", line 226, in async_handle_lock_service await self.async_lock(**self.add_default_code(kwargs)) File "/usr/src/homeassistant/homeassistant/components/matter/lock.py", line 90, in async_lock await self.send_device_command( File "/usr/src/homeassistant/homeassistant/components/matter/lock.py", line 70, in send_device_command await self.matter_client.send_device_command( File "/usr/local/lib/python3.12/site-packages/matter_server/client/client.py", line 454, in send_device_command return await self.send_command( ^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/matter_server/client/client.py", line 583, in send_command return await future ^^^^^^^^^^^^ matter_server.common.errors.UnknownError: InteractionModelError: UnsupportedAccess (0x7e)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcelveldt marcelveldt marcelveldt approved these changes

Assignees
No one assigned
Labels
bugfix Pull request that fixes a (known) issue/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Matter - Device state not updating
3 participants
@agners @Drealine @marcelveldt